Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Support more secret volume fields in secure settings #1665

Merged
merged 8 commits into from
Sep 4, 2019
Merged

Support more secret volume fields in secure settings #1665

merged 8 commits into from
Sep 4, 2019

Conversation

thbkrkr
Copy link
Contributor

@thbkrkr thbkrkr commented Aug 29, 2019

Support items, key and path fields for the secure
settings secret similar to secret volume mounts to
give a way to inject a subset of secrets and/or
define a specific name to secure settings.

New syntax supported:

spec:
  secureSettings:
  - secretName: your-secure-settings-secret
    items:
    - key: username
      path: my-login
  - secretName: your-second-secure-settings-secret

Resolves #1458.

@thbkrkr
Support more secret volume fields in secure settings
d2923af 
Support items, key and path fields for the secure
settings secret similar to secret volume mounts to
give a way to inject a subset of secrets and/or
define a specific name to secure settings.

New syntax supported:
```
spec:
  secureSettings:
  - secretName: your-secure-settings-secret
    items:
    - key: username
      path: my-login
  - secretName: your-second-secure-settings-secret
```
@thbkrkr thbkrkr added >enhancement Enhancement of existing functionality v1.0.0-beta1 labels Aug 29, 2019
pebrc
pebrc approved these changes Aug 30, 2019
View reviewed changes
Copy link
Collaborator

@pebrc pebrc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! Nice, much easier than I thought. One tiny naming nit.

pkg/controller/common/keystore/user_secret.go Outdated Show resolved Hide resolved
@barkbay
Copy link
Contributor

barkbay commented Sep 2, 2019

I wanted to test this feature but I have the following error when I apply the manifest:

validation failure list:
type in spec.secureSettings is required

I have deleted and recreated the CRDs. I'm using K8S 1.15.3 (both client and server).

Here is my manifest:

apiVersion: elasticsearch.k8s.elastic.co/v1alpha1
kind: Elasticsearch
metadata:
  name: elasticsearch-sample
spec:
  secureSettings:
  - secretName: some-user-secret
    items:
    - key: key1
  version: 7.2.0
  nodes:
  - name: default
    config:
      # most Elasticsearch configuration parameters are possible to set, e.g:
      node.attr.attr_name: attr_value
      node.master: true
      node.data: true
      node.ingest: true
      node.ml: true
    podTemplate:
      spec:
        containers:
        - name: elasticsearch
          # specify resource limits and requests
          resources:
            limits:
              memory: 4Gi
              cpu: 1
          env:
          - name: ES_JAVA_OPTS
            value: "-Xms2g -Xmx2g"
    nodeCount: 2

Any idea ?

Also we should create a documentation issue and/or update the comments in the sample.

@thbkrkr
Copy link
Contributor Author

thbkrkr commented Sep 2, 2019

I wanted to test this feature but I have the following error when I apply the manifest:

validation failure list:
type in spec.secureSettings is required

Thanks for spotting this!

items requires to declare a type, for the moment.

spec:
  secureSettings:
  - secretName: some-user-secret
    items:
    - key: key1
    type: array

It looks like this issue kubernetes/kubernetes#68466, which has been fixed for 1.16.

Also we should create a documentation issue and/or update the comments in the sample.

Yes! I'm going to update the doc and the samples in this PR.

@barkbay
Copy link
Contributor

barkbay commented Sep 2, 2019

items requires to declare a type, for the moment.

I added this one in the spec, it has fixed the issue with kubectl, but now I have the same error on the operator side:

2019-09-02T13:34:41.610+0200	ERROR	kubebuilder.controller	Reconciler error	{"controller": "elasticsearch-controller", "request": "default/elasticsearch-sample", "error": "Elasticsearch.elasticsearch.k8s.elastic.co \"elasticsearch-sample\" is invalid: []: Invalid value: map[string]interface {}{\"apiVersion\":\"elasticsearch.k8s.elastic.co/v1alpha1\", \"kind\":\"Elasticsearch\", \"metadata\":map[string]interface {}{\"annotations\":map[string]interface {}{\"common.k8s.elastic.co/controller-version\":\"0.0.0\", \"kubectl.kubernetes.io/last-applied-configuration\":\"{\\\"apiVersion\\\":\\\"elasticsearch.k8s.elastic.co/v1alpha1\\\",\\\"kind\\\":\\\"Elasticsearch\\\",\\\"metadata\\\":{\\\"annotations\\\":{},\\\"name\\\":\\\"elasticsearch-sample\\\",\\\"namespace\\\":\\\"default\\\"},\\\"spec\\\":{\\\"nodes\\\":[{\\\"config\\\":{\\\"node.attr.attr_name\\\":\\\"attr_value\\\",\\\"node.data\\\":true,\\\"node.ingest\\\":true,\\\"node.master\\\":true,\\\"node.ml\\\":true},\\\"name\\\":\\\"default\\\",\\\"nodeCount\\\":2,\\\"podTemplate\\\":{\\\"spec\\\":{\\\"containers\\\":[{\\\"env\\\":[{\\\"name\\\":\\\"ES_JAVA_OPTS\\\",\\\"value\\\":\\\"-Xms2g -Xmx2g\\\"}],\\\"name\\\":\\\"elasticsearch\\\",\\\"resources\\\":{\\\"limits\\\":{\\\"cpu\\\":1,\\\"memory\\\":\\\"4Gi\\\"}}}]}}}],\\\"secureSettings\\\":[{\\\"items\\\":[{\\\"key\\\":\\\"key1\\\"}],\\\"secretName\\\":\\\"some-user-secret\\\",\\\"type\\\":\\\"array\\\"}],\\\"version\\\":\\\"7.2.0\\\"}}\\n\"}, \"creationTimestamp\":\"2019-09-02T11:34:19Z\", \"generation\":2, \"name\":\"elasticsearch-sample\", \"namespace\":\"default\", \"resourceVersion\":\"176973\", \"selfLink\":\"/apis/elasticsearch.k8s.elastic.co/v1alpha1/namespaces/default/elasticsearches/elasticsearch-sample\", \"uid\":\"d9495903-cc0a-410b-918f-8985b83121d7\"}, \"spec\":map[string]interface {}{\"http\":map[string]interface {}{\"service\":map[string]interface {}{\"metadata\":map[string]interface {}{\"creationTimestamp\":interface {}(nil)}, \"spec\":map[string]interface {}{}}, \"tls\":map[string]interface {}{\"certificate\":map[string]interface {}{}}}, \"nodes\":[]interface {}{map[string]interface {}{\"config\":map[string]interface {}{\"node.attr.attr_name\":\"attr_value\", \"node.data\":true, \"node.ingest\":true, \"node.master\":true, \"node.ml\":true}, \"name\":\"default\", \"nodeCount\":2, \"podTemplate\":map[string]interface {}{\"metadata\":map[string]interface {}{\"creationTimestamp\":interface {}(nil)}, \"spec\":map[string]interface {}{\"containers\":[]interface {}{map[string]interface {}{\"env\":[]interface {}{map[string]interface {}{\"name\":\"ES_JAVA_OPTS\", \"value\":\"-Xms2g -Xmx2g\"}}, \"name\":\"elasticsearch\", \"resources\":map[string]interface {}{\"limits\":map[string]interface {}{\"cpu\":\"1\", \"memory\":\"4Gi\"}}}}}}}}, \"secureSettings\":[]interface {}{map[string]interface {}{\"items\":[]interface {}{map[string]interface {}{\"key\":\"key1\", \"path\":\"\"}}, \"secretName\":\"some-user-secret\"}}, \"updateStrategy\":map[string]interface {}{}, \"version\":\"7.2.0\"}}: validation failure list:\ntype in spec.secureSettings is required"}

@thbkrkr
Copy link
Contributor Author

thbkrkr commented Sep 2, 2019

My bad, indeed type is not enough. Unfortunately I do not see a way to fix or circumvent the validation failure caused by the presence of the items field.

I propose to use our own struct to type secureSettings, that has an entries field instead of items:

spec:
  secureSettings:
  - secretName: some-secret
  - secretName: another-secret
    entries:
    - key: key1
    - key: key2
      path: newkey2

Note that several fields of SecretVolumeSource have no sense in the context of the secure settings (optional, defaultMode, mode).

@thbkrkr thbkrkr force-pushed the support-items-key-path-in-securesettings branch from 9391fcb to 18f83f9 Compare September 3, 2019 13:40
@thbkrkr thbkrkr force-pushed the support-items-key-path-in-securesettings branch from 18f83f9 to a4bb10d Compare September 3, 2019 13:41
barkbay
barkbay approved these changes Sep 4, 2019
View reviewed changes
Copy link
Contributor

@barkbay barkbay left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 👍

pkg/controller/common/keystore/user_secret.go Outdated Show resolved Hide resolved
@thbkrkr thbkrkr merged commit abfcda3 into elastic:master Sep 4, 2019
@thbkrkr thbkrkr deleted the support-items-key-path-in-securesettings branch September 9, 2019 09:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@barkbay barkbay barkbay approved these changes

@pebrc pebrc pebrc approved these changes

Assignees
No one assigned
Labels
>enhancement Enhancement of existing functionality v1.0.0-beta1
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Support more secret volume fields in secure settings
3 participants
@thbkrkr @barkbay @pebrc